Upgrade rose edit to Python 3 #2808

astroDimitrios · 2024-09-10T14:48:56Z

As discussed this change updates rose config-edit (or just rose edit) to Python 3 / Gtk3.

First commit having run 2to3 and Pygobject over the old code 6a49dd7
Same for the gtk dir 8bccdcf
Splash screen updates 20ade08
Popup fixes in the MenuWidget 807bc56
Fix macro menus ed0780e

All seems to function par some cosmetic bugs which are outlined in the Issues on my fork.

oliver-sanders · 2024-10-31T11:07:08Z

Wow!

oliver-sanders

Have made a first pass over the code, looking good.

There's 186 commits here! We could do with squashing these down a bit to make it a bit more manageable.

metomi/rose/gtk/dialog.py

metomi/rose/gtk/util.py

metomi/rose/config_editor/status.py

metomi/rose/gtk/splash.py

metomi/rose/etc/rose-config-edit/style.css

metomi/rose/resource.py

oliver-sanders · 2024-10-31T13:06:53Z

metomi/rose/config_editor/valuewidget/array/entry.py

+        val_array = []
+        # Prevent str without "" breaking the underlying Python syntax
+        for e in self.entries:
+            v = e.get_text()
+            if v in ("False", "True"): # Boolean
+                val_array.append(v)
+            elif (len(v) == 0) or (v[:1].isdigit()): # Empty or numeric
+                val_array.append(v)
+            elif not v.startswith('"'): # Str - add in leading and trailing "
+                val_array.append('"' + v + '"')
+                e.set_text('"' + v + '"')
+                e.set_position(len(v)+1)
+            elif (not v.endswith('"')) or (len(v) == 1): # Str - add in trailing "
+                val_array.append(v + '"')
+                e.set_text(v + '"')
+                e.set_position(len(v))
+            else:


@oliver-sanders, please go through this.

metomi/rose/config_editor/nav_panel_menu.py

metomi/rose/config_editor/__init__.py

oliver-sanders · 2024-12-10T14:42:50Z

There are some flaky tests lurking in the battery at the moment (nothing to do with this PR), the following Mac OS failures can be ignored:

02-360day-cycling
13-app-arch-cmd-out
30-app-arch-opt-source

oliver-sanders

Sorry for the delay, back looking at this...

Have tested this out on Mac OS using conda install gtk3 pygobject, worked a charm. Much, much easier to install than the old version 🚀

Anyway looking good 👍

I did find some issues during testing, a couple of tracebacks and some minor things (some of which might just go on the issue tracker for another day)....

1) The "file" sections don't seem to be working:

I did hit this issue when I tried to open any of the file sections:

2) The right-click menu for "duplicate" items is too narrow and has broken icons

Example: demo-meta/10-duplicate/namelist/strawberry icecream

Spotted one minor thing with "duplicate" items (see example in the demo metadata), where the right click menu opened too narrow to display the whole text:

The icons seem to be scrambled too.

3) Multi-line text fields raise traceback when edited

Example: demo-meta/01-types/namelist/manytypes/my_raw_with_newlines

4) Focusing on a long text box causes horizontal scrolling

Example: demo-meta/01-types/namelist/many types/my_char

Before clicking on the text box:

After clicking on the text box (Rose 2019 didn't scroll horizontally in this situation):

5) char types get quotes added when focused

Example: demo-meta/01-types/namelist/many types/my_char_array

Before I click on the text box, there are no quote symbols in it. After I click in the text box, quote symbols appear:

(left: this PR, right: deployed Rose 2019)

I need to double check this one as I have some vague memory that we changed something to do with the quotes for char types at some point...

6) Scrolls to the top of the page when warnings are raised on field change

Example: demo-meta/01-types/namelist/many types/my_int_range

Before changing the value from 2->3 (by pressing the increment button):

After pressing the increment button (scrolls to the top of the page):

7) Raw strings don't change appearance when env vars are present

Example: demo-meta/01-types/namelist/many types/my_raw

(left: this PR, right: deployed Rose 2019)

The field used to go purple to show that an environment variable is present. This is useful information for the user as some checks cannot be performed when env vars are present.

8) Bug fixed: adding an item to a char array caused a crash

Example: demo-meta/01-types/namelist/many types/my_repeat_char_array

I spotted one bug in the old GUI that has been fixed on this branch 👏, thanks! Adding an item to the array caused a crash!

9) Clicking anywhere in my_repeat_real_array causes it to be flagged as an error

Example: demo-meta/01-types/namelist/many types/my_real_repeat_array

10) Widget changed for multiple select

Example: demo-meta/01-types/namelist/many types/my_values_lots_toggle

Used to be a dropdown, now it's a radio button list.

astroDimitrios · 2025-01-14T14:52:35Z

The above commits fix 1, 3, and 9.

wxtim · 2025-01-16T10:39:04Z

@astroDimitrios - simple de-confliction required.

wxtim · 2025-01-16T13:00:37Z

Note

Not finished, will update

Tim's Functional Review

New Bugs

Numbering cont'd from Oliver's bugs.

11. Annoying quotes

Severity: Likely to cause complaint, but not fatal. Concerned about how systematic this is.

(Bit like Oliver's bug 5.)
affects demo-meta/01-types/namelist/many types/:

my_int_array
my_int_array_long
my_real_array
my_repeat_int_array
my_repeat_real_array
my_spaced_list
my_values_few_array
my_values_lots_array

If one replaces the value with . (may or may not be reasonable) or any other typo non-number char you get ".". You cannot delete the quotation marks but by selecting the whole box and replacing the content will values. Backspace and Del do not work.

12. `double_quoted_string_with_backslash`

Severity: Severe for anyone who uses these.

Unclear from the docs whether this is a bug, but it certainly doesn't happen in the old version:

Try to change my_quoted to

Nothing
Something
Something with \n, \" or \\ in.

As an additional fun thing, deleting a quote at the start of such a string can cause a new quote at both ends - "like so""

13. Shortening fixed len arrays

Priority: Lowish - can be a new issue.

01-types/namelist/table_nl/my_boolean_array and my_real_array can be shortened with the - button:

Interestingly the old GUI will allow you to increase the length of the list if you've manually edited it to be too short, but never to shorten it.

14. Revert to saved => traceback

Priority - if users use this, very bad.

Change something and try Page => <any item>:

I tried crudely doing

diff --git a/metomi/rose/config_editor/main.py b/metomi/rose/config_editor/main.py
index 4715ff6c..82f841e5 100644
--- a/metomi/rose/config_editor/main.py
+++ b/metomi/rose/config_editor/main.py
@@ -734,12 +734,6 @@ class MainController(object):
                 self.menubar, add_menuitem, self._get_current_page()
             ),
         )
-        page_menu.get_submenu().connect(
-            "deactivate",
-            lambda m: self.main_handle.clear_page_menu(
-                self.menubar, add_menuitem
-            ),
-        )
         self.main_handle.load_macro_menu(self.menubar)
         self.update_bar_widgets()
         self.top_menu = self.menubar.uimanager.get_widget("/TopMenuBar")
diff --git a/metomi/rose/config_editor/menu.py b/metomi/rose/config_editor/menu.py
index a19496c4..23cd3c15 100644
--- a/metomi/rose/config_editor/menu.py
+++ b/metomi/rose/config_editor/menu.py
@@ -637,10 +637,6 @@ class MainMenuHandler(object):
         self.reporter.report(info_text, kind=kind)
         return error_count
 
-    def clear_page_menu(self, menubar, add_menuitem):
-        """Clear all page add variable items."""
-        add_menuitem.remove_submenu()
-
     def load_page_menu(self, menubar, add_menuitem, current_page):
         """Load the page add variable items, if any."""
         if current_page is None:

Which seemed to work without side effects - do you have a GTK2-3 reference which might help me understand why this method has gone.

15. Similar to 14, but with `Metadata => upgrade`

Priority - if users use this, very bad.

I think that this can be fixed with

diff --git a/metomi/rose/config_editor/upgrade_controller.py b/metomi/rose/config_editor/upgrade_controller.py
index 5137314f..19bb5488 100644
--- a/metomi/rose/config_editor/upgrade_controller.py
+++ b/metomi/rose/config_editor/upgrade_controller.py
@@ -51,7 +51,7 @@ class UpgradeController(object):
             Gtk.ResponseType.ACCEPT,
         )
         self.window = Gtk.Dialog(buttons=buttons)
-        self.set_transient_for(parent_window)
+        self.window.set_transient_for(parent_window)
         self.window.set_title(metomi.rose.config_editor.DIALOG_TITLE_UPGRADE)
         self.config_dict = {}
         self.config_directory_dict = {}
@@ -112,9 +112,9 @@ class UpgradeController(object):
             else:
                 cell = Gtk.CellRendererText()
             if i == len(columns) - 1:
-                column.pack_start(cell, True, True, 0)
+                column.pack_start(cell, True)
             else:
-                column.pack_start(cell, False, True, 0)
+                column.pack_start(cell, False)
             column.set_cell_data_func(cell, self._set_cell_data, i)
             self.treeview.append_column(column)
         self.treeview.connect("cursor-changed", self._handle_change_cursor)
@@ -155,7 +155,7 @@ class UpgradeController(object):
         self.window.set_focus(self.ok_button)
         self._set_ok_to_upgrade()
         max_size = metomi.rose.config_editor.SIZE_MACRO_DIALOG_MAX
-        my_size = self.window.size_request()
+        my_size = self.window.get_size()
         new_size = [-1, -1]
         extra = 2 * metomi.rose.config_editor.SPACING_PAGE
         for i in [0, 1]:

(and I've added these changes as suggestions to the PR too, if you prefer, alongside an attempt to find docs).

However, this leads to the tickbox

populate all possible versions crashing. I suspect that the output of

rose/lib/python/rose/config_editor/upgrade_controller.py

Line 208 in 2c8956a

path = self.treeview.get_cursor()[0]

may have changed.

16. Single char focus stealing

Priority: Low - it's annoying, but shouldn't block release

Found on suite info section, all three text boxes - On deletion of the last char, or insertion of a single char, the focus moves to the cog button.

(also seen in 01-types/namelist/many types/my-derived second element·)

Note

This may be related to Number 6

17. Failure on invalid `widget[rose-config-edit]` value

Priority: Can be spun out - shouldn't be a problem if Suite developers don't mess up or don't use this setting.

Replicate -

In etc/demo/rose-config-edit/demo_meta/app/01-types/meta/rose-meta.conf replace widget[rose-config-edit]=metomi.rose.config_editor.valuewidget.radiobuttons.RadioButtonsValueWidget with a garbage value (I was trying to check coverage by using widget[rose-config-edit]=metomi.rose.config_editor.valuewidget.choice.ChoicesValueWidget.

Try opening this config in the editor. In the old version nothing much happens differently in the new version the whole gui freezes.

Alternative standalone replication:

#!/bin/bash
TMP=$(mktemp -d)
echo Files extracted to ${TMP}
mkdir "${TMP}/."

mkdir "${TMP}/./meta"

cat > "${TMP}/./meta/rose-meta.conf" <<__ICI__
[namelist:one=foo]
widget[rose-config-edit]=metomi.rose.config_editor.valuewidget.choice.ChoicesValueWidget
                        =--editable__ICI__

cat > "${TMP}/./rose-app.conf" <<__ICI__
[namelist:one]
foo=3,3,3__ICI__

cd ${TMP}
rose edit -C examples/one/

should return

[FAIL] Could not import widget: Gtk.Box.pack_start() takes exactly 5 non-keyword arguments (2 given)

No obvious sign of line numbers, but I'd guess it'd be in

metomi.rose.config_editor.valuewidget.choice.ChoicesValueWidget

…e config-edit and an alias for edit.

…rsion script on Rose 2019.

Fixes issue with duplicate ns sorting app 10

If either section or option in the namespace (ns) are None then this doesn't sort properly with other values. This change ensures None is converted to a string to compare with normal string section/option values.

Fixes invisible text in the stash diag panel

…text "Search"

…se-edit

…le for Rose 2.0 Removes the extra rose config-edit section from the rose api docs and prevented the splash-screen entry points getting into the docs

… rose help

wxtim · 2025-01-16T15:31:09Z

metomi/rose/config_editor/upgrade_controller.py

+            Gtk.ResponseType.ACCEPT,
+        )
+        self.window = Gtk.Dialog(buttons=buttons)
+        self.set_transient_for(parent_window)


Relates to bug 15:

Rose 2019 code

Suggested change

self.set_transient_for(parent_window)

self.window.set_transient_for(parent_window)

wxtim · 2025-01-16T15:36:23Z

metomi/rose/config_editor/upgrade_controller.py

+                column.pack_start(cell, True, True, 0)
+            else:
+                column.pack_start(cell, False, True, 0)


I think that pack_start only takes 3 vars (including self.)

These extra variables arent in the old code and don't look like they match the reference material

For long term reference, @astroDimitrios explained to me that pack_start can have 2 or 4 (non self) args depending on which widget the message is attached to. 😢

wxtim · 2025-01-16T15:50:26Z

metomi/rose/config_editor/upgrade_controller.py

+        self.window.set_focus(self.ok_button)
+        self._set_ok_to_upgrade()
+        max_size = metomi.rose.config_editor.SIZE_MACRO_DIALOG_MAX
+        my_size = self.window.size_request()


I'm struggling to navigate the GTK docs, but I don't think get_size_request returns the same type as it did at GTK2. I'm also not clear, but suspect it's marked for deprecation.

Suggested change

my_size = self.window.size_request()

my_size = self.window.get_size()

wxtim · 2025-01-17T09:12:08Z

Works nicely on Ubuntu 22.04/Gnome 3.36.8/Solarized Dark Theme.

wxtim · 2025-01-17T10:10:40Z

@oliver-sanders - I'm not sure number 10 is correct: I think that something has gone wrong in the old version given that the metadata explicitly sets the toggle to radio buttons. What's worse, I can't get the radio buttons like you have on either new or old vdi.

widget[rose-config-edit]=metomi.rose.config_editor.valuewidget.radiobuttons.RadioButtonsValueWidget

If you remove this line you do get a dropdown box in the new version.

wxtim · 2025-01-17T10:11:52Z

Bug Summary

Includes recapitulation of Olivers bugs

Have they been fixed? What priority?

Bug	Fixed?	How bad?
1	Yes
2	No	High priority to fix.
3	Yes
4	No	Can be lived with
5	No	Can be lived with, but related bug, 11 is much more of an issue
6	No	Very Frustrating, should fix
7	No	Nice to have only IMO
8	No	Can't replicate - could have been fixed by the fix for 9?
9	Yes	:)
10	No	Couldn't replicate, not convinced it exists?
11	No	Not great.
12	No	Annoying, but should not be allowed to block PR for prolonged period
13	No	Should be spun into a new issue
14	No	probably needs fixing
15	No	probably needs fixing
16	No	Should be spun into a new issue
17	No	Should be turned into a new issue (should only affect metadata authors)

wxtim · 2025-01-17T15:14:48Z

metomi/rose/config_editor/valuewidget/choice.py

+        list_frame.show()
+        list_frame.add(self._listview)
+        list_vbox.pack_start(list_frame, expand=False, fill=False, padding=0)
+        self.pack_start(list_vbox, expand=True, fill=True)


might need

Suggested change

self.pack_start(list_vbox, expand=True, fill=True)

self.pack_start(list_vbox, expand=True, fill=True, padding=0)

wxtim · 2025-01-17T15:21:49Z

metomi/rose/config_editor/valuewidget/choice.py

+
+    def _get_add_widget(self):
+        add_hbox = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL)
+        add_entry = Gtk.ComboBoxEntry()


This line fails when https://github.com/metomi/rose/pull/2808/files#r1920339780 is fixed. 😢

Looks like some of the GTK interfaces have changed.

(Relates to Bug 17)

wxtim · 2025-01-17T15:26:32Z

How I'm looking at coverage

The following diff allows us to monitor which code manual testing is hitting:

diff.txt

To use

> export CYLC_COVERAGE=1
> rose edit -C path/to/app/or/suite
> # Do some manual testing, then close the GUI
> coverage combine
> coverage html
> firefox htmlcov/index.html

oliver-sanders added this to the 2.4.0 milestone Sep 11, 2024

oliver-sanders added the UI label Sep 11, 2024

oliver-sanders assigned astroDimitrios Sep 11, 2024

astroDimitrios marked this pull request as ready for review October 29, 2024 15:58

oliver-sanders self-requested a review October 31, 2024 11:06

oliver-sanders reviewed Oct 31, 2024

View reviewed changes

jfrost-mo mentioned this pull request Oct 31, 2024

Add James Frost to contributors #2823

Merged

astroDimitrios force-pushed the feature/rose-config-py3 branch 3 times, most recently from 5684ff9 to 4cf6267 Compare November 27, 2024 13:11

J-J-Abram force-pushed the feature/rose-config-py3 branch from d7fb376 to 07cd350 Compare November 27, 2024 15:36

astroDimitrios force-pushed the feature/rose-config-py3 branch 4 times, most recently from c5a6ee7 to edd7db0 Compare November 27, 2024 17:54

astroDimitrios requested a review from oliver-sanders November 27, 2024 18:05

oliver-sanders modified the milestones: 2.4.0, 2.5.0 Dec 3, 2024

astroDimitrios force-pushed the feature/rose-config-py3 branch from 8d1aef1 to f345292 Compare December 10, 2024 14:15

oliver-sanders reviewed Jan 6, 2025

View reviewed changes

astroDimitrios force-pushed the feature/rose-config-py3 branch 2 times, most recently from 067084a to 44e54eb Compare January 16, 2025 13:33

astroDimitrios added 3 commits January 16, 2025 13:37

Remove dead_ends for config-edit and edit. Add in entry point for ros…

9c6b6e7

…e config-edit and an alias for edit.

Add config_editor dir created by running 2to3 and the pygobject conve…

52ce070

…rsion script on Rose 2019.

Convert imports and usage from rose. to metomi.rose.

83825d0

astroDimitrios and others added 20 commits January 16, 2025 13:37

Fixes sort calls removing cmp usage

8b6c488

Fixes issue with duplicate ns sorting app 10

Fixes macro button and submenu

555b394

Fix to deal with comparison of NoneTypes and strings

9890fa8

If either section or option in the namespace (ns) are None then this doesn't sort properly with other values. This change ensures None is converted to a string to compare with normal string section/option values.

Removing all references to pygtk (#41)

0c8a1ad

Fixes popup() calls to GTK3 equivalents

e558faa

Fixes to the UM stash panel

6849005

Fixes invisible text in the stash diag panel

Update importlib code when importing custom widgets in resource.py

2529332

Change search box from an Entry to a SearchEntry box and add default …

fc7a0b6

…text "Search"

Add pygobject as a rose-edit dependency for install (#35)

ec3a11e

Re-enable plotting of Metadata graphs

760ec70

Tutorial username file updated to GTK3 and Python3 (#45)

1fb0219

Update setter functions to fix python list formatting (#46)

48b8158

Black formats the gui code and limits line length

a2bd3da

Fixes MyPy error in data.py where the exc variable was being overwritten

3ef6e8d

Updates the test workflow to install the required dependencies for ro…

b7b6f00

…se-edit

Removes references in the README and the docs to no GUI being availab…

a7ce70a

…le for Rose 2.0 Removes the extra rose config-edit section from the rose api docs and prevented the splash-screen entry points getting into the docs

Prevents the launch-splash-screen internal cli being shown when using…

924d306

… rose help

Fixes file chooser widget import

d520f04

Fixes multiline text box usage of Gtk.TextBuffer.get_text()

61ae825

Fixes repeat real array bug where numbers were turned into strings

3704bd5

astroDimitrios force-pushed the feature/rose-config-py3 branch from 44e54eb to 3704bd5 Compare January 16, 2025 13:43

wxtim reviewed Jan 16, 2025

View reviewed changes

wxtim reviewed Jan 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade rose edit to Python 3 #2808

Upgrade rose edit to Python 3 #2808

astroDimitrios commented Sep 10, 2024 •

edited

Loading

oliver-sanders commented Oct 31, 2024

oliver-sanders left a comment

oliver-sanders Oct 31, 2024

oliver-sanders commented Dec 10, 2024

oliver-sanders left a comment •

edited

Loading

astroDimitrios commented Jan 14, 2025

wxtim commented Jan 16, 2025

wxtim commented Jan 16, 2025 •

edited

Loading

wxtim Jan 16, 2025

wxtim Jan 16, 2025

wxtim Jan 17, 2025

wxtim Jan 16, 2025

wxtim commented Jan 17, 2025

wxtim commented Jan 17, 2025

wxtim commented Jan 17, 2025 •

edited

Loading

wxtim Jan 17, 2025

wxtim Jan 17, 2025 •

edited

Loading

wxtim commented Jan 17, 2025

	self.set_transient_for(parent_window)
	self.window.set_transient_for(parent_window)

	my_size = self.window.size_request()
	my_size = self.window.get_size()

	self.pack_start(list_vbox, expand=True, fill=True)
	self.pack_start(list_vbox, expand=True, fill=True, padding=0)

Upgrade rose edit to Python 3 #2808

Are you sure you want to change the base?

Upgrade rose edit to Python 3 #2808

Conversation

astroDimitrios commented Sep 10, 2024 • edited Loading

oliver-sanders commented Oct 31, 2024

oliver-sanders left a comment

Choose a reason for hiding this comment

oliver-sanders Oct 31, 2024

Choose a reason for hiding this comment

oliver-sanders commented Dec 10, 2024

oliver-sanders left a comment • edited Loading

Choose a reason for hiding this comment

1) The "file" sections don't seem to be working:

2) The right-click menu for "duplicate" items is too narrow and has broken icons

3) Multi-line text fields raise traceback when edited

4) Focusing on a long text box causes horizontal scrolling

5) char types get quotes added when focused

6) Scrolls to the top of the page when warnings are raised on field change

7) Raw strings don't change appearance when env vars are present

8) Bug fixed: adding an item to a char array caused a crash

9) Clicking anywhere in my_repeat_real_array causes it to be flagged as an error

10) Widget changed for multiple select

astroDimitrios commented Jan 14, 2025

wxtim commented Jan 16, 2025

wxtim commented Jan 16, 2025 • edited Loading

Tim's Functional Review

New Bugs

11. Annoying quotes

12. double_quoted_string_with_backslash

13. Shortening fixed len arrays

14. Revert to saved => traceback

15. Similar to 14, but with Metadata => upgrade

16. Single char focus stealing

17. Failure on invalid widget[rose-config-edit] value

Replicate -

wxtim Jan 16, 2025

Choose a reason for hiding this comment

wxtim Jan 16, 2025

Choose a reason for hiding this comment

wxtim Jan 17, 2025

Choose a reason for hiding this comment

wxtim Jan 16, 2025

Choose a reason for hiding this comment

wxtim commented Jan 17, 2025

wxtim commented Jan 17, 2025

wxtim commented Jan 17, 2025 • edited Loading

Bug Summary

wxtim Jan 17, 2025

Choose a reason for hiding this comment

wxtim Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

wxtim commented Jan 17, 2025

How I'm looking at coverage

astroDimitrios commented Sep 10, 2024 •

edited

Loading

oliver-sanders left a comment •

edited

Loading

wxtim commented Jan 16, 2025 •

edited

Loading

12. `double_quoted_string_with_backslash`

15. Similar to 14, but with `Metadata => upgrade`

17. Failure on invalid `widget[rose-config-edit]` value

wxtim commented Jan 17, 2025 •

edited

Loading

wxtim Jan 17, 2025 •

edited

Loading